Skip to content

feat(life_capture): Composio → life_capture bridge for Gmail + Calendar (A2) - #820

Closed
jwalin-shah wants to merge 41 commits into
tinyhumansai:mainfrom
jwalin-shah:feat/a2-composio-bridge
Closed

jwalin-shah wants to merge 41 commits into
tinyhumansai:mainfrom
jwalin-shah:feat/a2-composio-bridge

Conversation

@jwalin-shah

@jwalin-shah jwalin-shah commented Apr 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds the A2 piece of the v1 plan: a Composio event subscriber that ingests Gmail messages and Google Calendar events into the life_capture personal index.

  • New src/openhuman/life_capture/ingest/composio.rs defining LifeCaptureComposioBridge + EventHandler impl on the composio domain
  • Matches GMAIL_NEW_GMAIL_MESSAGE / GMAIL_NEW_MESSAGE / GOOGLECALENDAR_NEW_EVENT / GOOGLECALENDAR_EVENT_UPDATED
  • Dedupe via external_id (Gmail messageId / Calendar event id)
  • Reuses existing pick_str from composio::providers (no duplication)
  • Wired at tail of bootstrap_life_capture in src/core/jsonrpc.rs — existing register_composio_trigger_subscriber() is untouched
  • Idempotent registration via OnceLock

Dependency

⚠️ Stacked on #817 (feat/life-capture-foundation). Diff includes A1 (be1112f9 — life_capture ingest controller + iMessage scanner wiring) on top of the A2 work. Once #817 merges, rebase will drop A1 from this diff.

Test plan

  • Unit: per-trigger dispatch, no-op on unknown trigger, timestamp parser (s/ms/RFC-3339/YYYY-MM-DD), idempotent re-fire
  • Integration: tests/composio_ingest_e2e.rs publishes on the real global bus (#[ignore]'d so CI default run is unaffected — use cargo test -- --ignored for e2e)
  • cargo build --tests clean
  • cargo test --lib life_capture — 36 passed

Summary by CodeRabbit

  • New Features

    • Personal life capture index with hybrid search across email, calendar, and messages
    • Curated memory management for persistent MEMORY.md and USER.md files
    • iMessage ingestion support for personal data collection
    • Composio trigger integration for Gmail and Google Calendar automation
    • Installation script improvements with --source-only mode and better asset validation
  • Dependencies

    • Added SQLite connection pooling and vector database support
  • Documentation

    • New technical specification and implementation plans for life capture foundation and installation pipeline

@coderabbitai

coderabbitai Bot commented Apr 23, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@jwalin-shah has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 45 minutes and 5 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 61f8307c-9d8e-4dcd-bd40-8e473c9d0d06

📥 Commits

Reviewing files that changed from the base of the PR and between 999231d and 0edb670.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • .github/workflows/installer-smoke.yml
  • Cargo.toml
  • app/src-tauri/src/imessage_scanner/mod.rs
📝 Walkthrough

Walkthrough

This PR introduces a foundational "life capture" system for local personal data indexing via SQLite and sqlite-vec, implements a curated-memory snapshot mechanism for stable agent prompts, refactors the iMessage scanner to use the new ingest pathway, updates installation/release infrastructure, and extends the agent session harness to thread curated snapshots through prompt rendering and sub-agent execution.

Changes

Cohort / File(s) Summary
Installation/Release Pipeline
.github/workflows/installer-smoke.yml, scripts/install.sh, scripts/test_install.sh, scripts/fixtures/latest.json, docs/superpowers/plans/2026-04-22-track1-ship-pipeline.md
Adds explicit permissions: contents: read to workflow, restricts smoke test matrix to macOS, introduces --source-only sourcing mode and resolve_asset_url() refactoring in install script, creates new smoke test script that validates resolver fixtures, and adds comprehensive Track 1 pipeline/CI/updater implementation plan.
Dependencies
Cargo.toml
Adds runtime dependencies for SQLite pooling (r2d2, r2d2_sqlite) and vector support (sqlite-vec), plus test dependency (httpmock).
iMessage Scanner Migration
app/src-tauri/src/imessage_scanner/mod.rs, docs/superpowers/plans/2026-04-21-imessage-live-harness.md
Migrates iMessage ingestion from openhuman.memory_doc_ingest to openhuman.life_capture_ingest, reshapes JSON-RPC payload to use (source, external_id) keying with deterministic timestamp derivation from date string, updates metadata structure and logging. Plan doc updates Step 6 status and specifies verification via new harness.
Curated Memory System
src/openhuman/curated_memory/*, src/core/jsonrpc.rs (init section)
Introduces file-backed memory storage with MemoryFile enum, MemoryStore with async read/add/replace/remove operations bounded by char limits, MemorySnapshot for frozen-in-time snapshots, process-global CuratedMemoryRuntime initialization, RPC handlers (read/add/replace/remove), and controller schema/handler registration. Server startup initializes curated-memory stores and runtime.
Life Capture Foundation
src/openhuman/life_capture/*, src/core/jsonrpc.rs (init section)
Adds complete personal index subsystem: Item/Source/Query/Hit domain types, Embedder trait with HostedEmbedder HTTP implementation, PersonalIndex with IndexWriter/IndexReader for upsert/keyword/vector/hybrid search, SQLite schema migrations with FTS5 and sqlite-vec support, RPC handlers for get_stats/search/ingest, controller schemas, Composio ingest bridge for Gmail/Calendar normalization, quote-reply stripping and email/phone/SSN/CC redaction utilities, and LLM tool for hybrid search queries. Server startup opens index DB and initializes embedder when API keys present.
Agent Prompt Context Integration
src/openhuman/agent/harness/session/types.rs, src/openhuman/agent/harness/session/builder.rs, src/openhuman/agent/harness/session/turn.rs, src/openhuman/agent/harness/session/runtime.rs, src/openhuman/agent/harness/fork_context.rs, src/openhuman/agent/harness/subagent_runner/ops.rs, src/openhuman/agent/prompts/types.rs, src/openhuman/agent/prompts/mod.rs, src/openhuman/agent/triage/escalation.rs, src/openhuman/agent/triage/evaluator.rs, src/openhuman/agent/debug/mod.rs
Adds curated_snapshot optional field to Agent/ParentExecutionContext/PromptContext, captures snapshot once per session on first turn, threads it through subagent execution and prompt rendering, updates UserFilesSection::build to inject snapshot content from MemorySnapshot instead of workspace files when available, adds public accessor curated_snapshot() on Agent.
Agent Prompt Test Updates
src/openhuman/agent/agents/*/prompt.rs (17 files), src/openhuman/agent/agents/loader.rs, src/openhuman/learning/prompt_sections.rs
Updates all agent prompt builder unit tests to initialize new PromptContext.curated_snapshot: None field for compilation/consistency across all agent types (archivist, code_executor, critic, help, integrations_agent, loader, morning_briefing, orchestrator, planner, researcher, summarizer, tool_maker, tools_agent, trigger_reactor, trigger_triage, welcome).
Controller Registry
src/core/all.rs, src/openhuman/mod.rs, src/openhuman/tools/impl/mod.rs
Extends global controller registry to include life_capture and curated_memory namespaces in both registered handlers and declared schemas lists, introduces new module declarations for life_capture and curated_memory submodules, adds LifeCaptureSearchTool to agent tool registry.
End-to-End Tests
tests/composio_ingest_e2e.rs, tests/imessage_ingest_e2e.rs, src/openhuman/life_capture/tests/e2e.rs, tests/life_capture_retrieval_eval.rs, tests/agent_harness_public.rs, tests/fixtures/life_capture/corpus.json
Adds comprehensive test coverage: Composio webhook ingestion with idempotency verification, iMessage → life_capture_ingest validation with replaced-item tracking, in-memory index deterministic-embedder e2e pipeline, retrieval evaluation corpus and ranking constraint validation, all tests using isolated runtime/index instances and fixture-based deterministic embedders to avoid network deps.
Documentation Plans
docs/superpowers/plans/2026-04-22-life-capture-01-foundation.md, docs/superpowers/specs/2026-04-22-life-capture-layer-design.md
Foundation plan specifies Rust module structure, schema design (items/FTS/vectors/migrations), domain types, retrieval logic, privacy redaction, controller exposure, integration tests, and database initialization. Design spec outlines end-to-end architecture, privacy modes (Convenience/Hybrid/Fully local), app UI changes, Track 1 pipeline unblocking, and Track 2 week-by-week implementation milestones.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • PR #506 — Both add session-scoped metadata fields to Agent/AgentBuilder to stabilize prompt rendering across turns (this PR: curated_snapshot; #506: system_prompt_cache_boundary).
  • PR #724 — This PR refactors the iMessage scanner that PR #724 originally introduced, changing its ingest target and payload shape to align with the new life_capture foundation.
  • PR #732 — Both PRs extend the global controller registry in src/core/all.rs by registering new controller namespaces (this PR: life_capture/curated_memory; #732: memory_tree).

Suggested reviewers

  • senamakel

🐰 A personal index takes root today,
With SQLite vectors lighting the way,
Curated snapshots keep prompts quite stable,
While life flows in through each ingest table,
Gmail, iMessage, Calendar too—
All captured and searchable, just for you! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main feature: adding a Composio bridge for Gmail and Calendar ingestion into the life_capture system, with A2 denoting the specific phase in the implementation plan.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Drafts the wedge for OpenHuman: continuous ingestion of Gmail/Calendar/Slack/
iMessage into a local personal_index.db, with a Today brief and retrieval-
tuned chat as the two surfaces. Three composable privacy modes (Convenience /
Hybrid / Fully local) sharing one code path. Track 1 unblocks the ship
pipeline; Track 2 builds the spine over 4 weeks. Reuses ingestion logic from
~/projects/inbox.
Two implementation plans drafted from the life-capture spec:

- Track 1 (ship pipeline): fix Ubuntu installer smoke, land in-flight PRs
  tinyhumansai#806/tinyhumansai#786/tinyhumansai#788/tinyhumansai#797, wire Tauri auto-updater + signed Mac/Windows builds.
- Life-Capture #1 (foundation): SQLite + sqlite-vec personal_index.db,
  Embedder trait + HostedEmbedder (OpenAI), PII redaction, quoted-thread
  stripping, hybrid retrieval (vector + keyword + recency), controller
  schema + RPC. End-to-end test with synthetic items. No ingestion or UI
  yet — those are subsequent milestone plans.
Refactors scripts/install.sh to expose resolve_asset_url and
verify_asset_reachable. Adds scripts/test_install.sh that exercises
the resolver against a committed fixture latest.json. Failures now
report the resolved URL and the parse error instead of dying silently.
Adopts the Hermes pattern (NousResearch/hermes-agent, MIT) of an
agent-writable, char-bounded curated memory file pair. F15 builds the
char-bounded store with atomic writes; F16 wires a session-start
snapshot into the system prompt (preserves prefix cache); F17 exposes
memory.{add,replace,remove,read} through the controller dispatch so
both the agent loop and skills can use it.

Sits between TinyHumans synthesis (volatile) and personal_index.db
(raw events) — fills the "deliberately curated facts" gap that
neither covered.
Adds src/openhuman/life_capture/ module tree (stubs for embedder,
index, migrations, quote_strip, redact, rpc, schemas, types, plus
tests/) per Foundation Plan F1. Wires the module in src/openhuman/mod.rs.

Adds sqlite-vec to deps and httpmock as a dev-dep. Reuses existing
rusqlite/regex/once_cell/async-trait/tempfile already in tree
(Option C — avoids sqlx/rusqlite libsqlite3-sys conflict).
…egex

Previously the country-code group required at least one digit (\+?\d{1,3}),
so '(415) 555-0123' (no leading country code) wouldn't match. Wrapped the
whole prefix in a non-capturing optional group so parenthesized area codes
match without a leading country code.
… table

Loads sqlite_vec via sqlite3_auto_extension exactly once per process so every
new connection picks up the vec0 module. PersonalIndex wraps a single rusqlite
Connection in Arc<Mutex<_>> for async sharing; open() runs migrations and sets
WAL+foreign_keys, open_in_memory() is for tests.

vec_version() round-trip and a 1536-dim INSERT + MATCH query both verified.
…_id) dedupe

Adds IndexWriter with upsert (ON CONFLICT(source, external_id) DO UPDATE — FTS
stays in sync via triggers) and upsert_vector (DELETE + INSERT since vec0 has
no ON CONFLICT for virtual table primary keys). Both wrap rusqlite work in
spawn_blocking and share the PersonalIndex's Arc<Mutex<Connection>>.

Exposes ensure_vec_extension_registered() crate-wide so the migrations test
can register vec0 before opening its in-memory connection.
- keyword_search: FTS5 ranked by negated bm25 with snippet markers (« » …),
  plus the user:local ACL EXISTS clause so the same query shape works for the
  multi-token team v2 ACL without rewrites.
- vector_search: sqlite-vec MATCH with k = ? clause (vec0 KNN requirement)
  and ORDER BY distance; score = 1/(1 + distance) so callers can blend it
  with keyword bm25 on the same monotonic scale.

Shared ItemRow + into_hit() helper lets both queries reuse the same row
shape; rusqlite query_map closures hand-build it because rusqlite has no
sqlx FromRow equivalent.
Pulls oversampled candidates (k*3 min 20) from both keyword and vector legs,
normalises each independently to [0,1], then re-ranks with
0.55 * vec_norm + 0.35 * kw_norm + 0.10 * exp(-age_days/30).

Documents present in only one leg get 0 for the missing signal but still
compete on the others; only items with neither signal are dropped. Same-vector
twin items now break their tie by recency.
…rch)

Adds two controllers exposed via the registry:
- life_capture.get_stats — total items, per-source counts, last-ingest ts.
- life_capture.search    — hybrid (vector + keyword + recency) search with
  embed-then-rank, optional k (default 10, capped at 100).

Runtime state (PersonalIndex + dyn Embedder) lives in life_capture::runtime
behind a tokio OnceCell because handlers are stateless fn(Map<String,Value>)
and have no per-call context object. F14 will call runtime::init() at app
startup; until then handlers return a structured 'not initialised' error so
the failure mode is loud, not silent.

Schemas registered in core/all.rs alongside cron.
… → search)

FakeEmbedder hashes input bytes into a deterministic sparse 1536-dim vector
so the same text round-trips identically through vec0 — keeps the e2e test
hermetic, no network call. Verifies that:
- quote-strip drops the 'On … wrote:' block before indexing,
- redact masks the email address before indexing,
- the upserted item resurfaces via hybrid_search with both transformations
  preserved on the returned text.
After Config::load_or_init succeeds, open the personal_index.db at
config.workspace_dir and register the life_capture runtime. Embedder is
env-gated:
  OPENHUMAN_EMBEDDINGS_KEY > OPENAI_API_KEY
  OPENHUMAN_EMBEDDINGS_URL (default: https://api.openai.com/v1)
  OPENHUMAN_EMBEDDINGS_MODEL (default: text-embedding-3-small)

If no key is set we still open the index file (so Plan #2's ingestion
worker can write to it) but skip runtime registration — controllers then
return the 'not initialised' error from runtime::get() instead of silently
calling a misconfigured embedder.

Config-schema integration deferred to a follow-up; env-driven keeps F14
non-invasive while we land the rest of the foundation.
…ection

snapshot_pair(memory, user) returns a MemorySnapshot containing plain
strings — no reference back to the stores — so taking a snapshot at session
start and reusing it across turns gives a stable system-prompt prefix and
lets the LLM prefix-cache hit on every subsequent turn.

Plan #1 also calls for wiring this into chat.rs' OpenClaw context loader,
but openhuman assembles agent prompts per-agent under
src/openhuman/agent/agents/*/prompt.rs (not the plan's stale
src-tauri/src/commands/chat.rs path); the prompt-builder integration is
deferred to Plan #2 alongside the agent-context refactor.
… controllers

Adds four controllers under the memory_curated namespace (the 'memory'
namespace is already owned by the long-term memory subsystem):
- memory_curated.read    — read MEMORY.md or USER.md
- memory_curated.add     — append entry, char-bounded
- memory_curated.replace — substring replace, char-bounded
- memory_curated.remove  — drop entries containing a needle

'file' input is a typed enum (memory|user) so adapters reject anything
else at validation time. Runtime state lives in curated_memory::runtime
behind a tokio OnceCell, mirroring life_capture's pattern: startup in
jsonrpc.rs constructs both stores at <workspace>/memories/ with Hermes'
char limits (memory: 2200, user: 1375) and registers the runtime.
Handlers return 'not initialised' until init runs, so failure is loud.
…ings

Findings from Codex + Gemini second-opinion review:

1. IndexWriter::upsert orphaned vectors on re-ingest (Codex). On
   (source, external_id) conflict the row's id was kept but the caller's
   fresh UUID was used for upsert_vector — the vector wrote under an id no
   row joined to. Fix: explicit SELECT-then-UPDATE-or-INSERT in the same
   transaction; mutates caller's Item.id to the canonical id (so the next
   upsert_vector lands on the right row) and orphan-deletes any vector
   already written under the wrong id. Signature change: upsert(&[Item])
   -> upsert(&mut [Item]).

2. upsert_vector DELETE+INSERT was outside a transaction (Gemini) — a
   failed INSERT permanently lost the item's vector. Now wrapped in tx.

3. Runtime over-gated get_stats on the embedder (Codex). Split runtime
   into separate INDEX + EMBEDDER OnceCells: get_stats only requires
   index, search requires both. Index initialises whenever the workspace
   dir is reachable; embedder is env-gated as before.

4. Startup race (both reviewers): runtime init lived inside the post-
   serve tokio::spawn block, so axum::serve was already accepting before
   the OnceCells were set. Hoisted both bootstraps (curated_memory and
   life_capture) into helpers called inline before axum::serve.

5. runtime::get error message lied (Codex) — said 'set embeddings.api_key
   in config' but startup actually reads env vars. Fixed text.

Bonus: rename controller namespace memory_curated -> curated_memory
(Codex preference; nothing depends on it yet so renamed before clients
do).

Adds regression test for #1 (reingest_with_fresh_uuid_keeps_vector_findable).
4696 lib tests pass.
…emory namespace rename

Pre-push hook auto-applied cargo fmt across the foundation files; also fixed the schemas.rs docstring still referencing the old memory_curated namespace.
Critical:
- curated_memory/store.rs: propagate read errors instead of `unwrap_or_default`,
  which could rewrite MEMORY.md / USER.md from an empty baseline on transient
  I/O or permission failures. Reject empty needle in replace/remove.
- curated_memory/rpc.rs: belt-and-suspenders empty-needle guard at the RPC
  boundary (remove with "" deletes every entry).
- life_capture/index.rs: upsert_vector rejects orphan ids (items row missing),
  which would have inserted vectors that never join in vector_search.
- life_capture/embedder.rs: validate response length matches input, indices
  are contiguous 0..n, and every vector matches dim() — prevents silent
  misalignment or wrong-dimensional vectors flowing into the 1536-wide
  sqlite-vec table. Added 30s request + 10s connect timeouts.

Retrieval hardening:
- life_capture/index.rs: keyword_search now escapes FTS5 operators by
  tokenizing on whitespace and wrapping each token as a quoted literal.
  Prevents errors or unintended matches from stray quotes, AND/OR/NEAR,
  column specifiers in user input.
- life_capture/index.rs: hybrid_search applies q.sources / q.since / q.until
  via post-filter (single consistent pass across keyword+vector fusion).
- life_capture/rpc.rs: handle_search validates embedder dim and response
  length against the fixed 1536-wide index column — clear RPC error instead
  of a cryptic sqlite-vec failure when the embeddings model is swapped.

Docs:
- docs/event-bus.md: remove erroneous `.await` on register_native_global
  (it is sync; the handler closure is async).
- life-capture design spec: soften PII redaction claim — only regex is
  implemented today; light NER is flagged as a future enhancement.
Rendered prompts now surface runtime curated-memory writes:

- `PromptContext` gains `curated_snapshot: Option<&MemorySnapshot>`.
- `UserFilesSection` prefers the snapshot over the workspace-file
  loader when one is attached, and injects `USER.md` alongside
  `MEMORY.md` using a byte-compatible `inject_snapshot_content` helper.
- `Session` carries `Option<Arc<MemorySnapshot>>`, populated by
  `ensure_curated_snapshot` on the first turn from
  `curated_memory::runtime::get()`. Reused across turns so prompt
  bytes stay frozen (KV-cache prefix contract) while mid-session
  `curated_memory.add/replace/remove` writes land on the NEXT session.
- `ParentExecutionContext` inherits the snapshot so sub-agents render
  identical `MEMORY.md`/`USER.md` blocks as the parent.
- Legacy workspace-file fallback preserved for embeds that don't
  initialise the curated-memory runtime (pure unit tests).
Loads a 32-item corpus across gmail/calendar/imessage/slack into an
in-memory PersonalIndex and runs 12 queries through keyword / vector /
hybrid paths, asserting must_contain / must_not_contain within per-query
top-K prefixes.

The vector leg uses a deterministic FNV-1a feature-hashing embedder
(1536 dims, L2-normalized) so the test is offline and reproducible; real
embedder swaps behind one call. Fixture reserves `relevant` and
`pending` fields so recall@k / MRR / nDCG bolt onto the same JSON later.

q-src-01 is marked pending: hybrid_search currently ignores
Query.sources/since/until. Flip to false once filtering lands.
Previously every read grabbed the same `Arc<Mutex<Connection>>` as the
writer, so `IndexReader` calls serialised behind in-flight ingests even
though WAL would allow parallel readers. Split `PersonalIndex` into:

- `writer: Arc<Mutex<Connection>>` — unchanged single-writer model.
- `reader_pool: Option<Arc<r2d2::Pool<SqliteConnectionManager>>>` —
  four-connection read pool on file-backed indexes, built alongside the
  writer after migrations run. Each pooled connection gets
  `query_only=ON` as a belt-and-suspenders guard; sqlite-vec is loaded
  automatically via the process-wide auto-extension.

`IndexReader` routes `keyword_search` / `vector_search` through a new
`with_read_conn` helper that picks the pool when present and falls
back to the writer lock otherwise. In-memory handles keep the
single-connection layout — shared-cache URIs buy nothing at
test-fixture scale and would force every test to grow a unique name.
- Add `life_capture.ingest` RPC controller (idempotent upsert by
  `(source, external_id)`, embeds text + atomic vector replace).
- Switch iMessage scanner's `ingest_group()` to POST
  `openhuman.life_capture_ingest` instead of `openhuman.memory_doc_ingest`,
  so chat-day transcripts land in the PersonalIndex (the same store
  `life_capture.search` reads from).
- Fix `life_capture.search` envelope: wrap hits in `{"hits": [...]}` per
  the schema (previously returned a bare array — obs 3165).
- New `#[ignore]`d e2e at `tests/imessage_ingest_e2e.rs` proves the bridge:
  ingest → search → re-ingest is idempotent (`total_items` unchanged,
  `replaced=true`).
Adds a second subscriber on the `composio` domain that normalises
Composio trigger webhooks into `life_capture::types::Item` and pushes
them through the existing `rpc::handle_ingest` path — so Gmail and
Google Calendar events land in the same PersonalIndex that iMessage
transcripts already populate, keyed for idempotent upsert by
`(source, external_id)`.

Files added:
- src/openhuman/life_capture/ingest/mod.rs (light export-only)
- src/openhuman/life_capture/ingest/composio.rs (bridge + registrar)
- tests/composio_ingest_e2e.rs (#[ignore]'d live-bus e2e)

Files modified:
- src/openhuman/life_capture/mod.rs (add `pub mod ingest;`)
- src/core/jsonrpc.rs (call `register_life_capture_composio_bridge`
  from the existing `bootstrap_life_capture` once index + embedder
  are both ready; the existing `ComposioTriggerSubscriber` for triage
  + history is left completely untouched)

Trigger handlers (case-insensitive match):
- GMAIL_NEW_GMAIL_MESSAGE / GMAIL_NEW_MESSAGE     → ingest_gmail
- GOOGLECALENDAR_NEW_EVENT                         → ingest_calendar
- GOOGLECALENDAR_EVENT_UPDATED                     → ingest_calendar
- other                                            → no-op

Dedupe key per source:
- Gmail:    payload `messageId` / `id` (tried in fallback order)
- Calendar: payload `id` / `eventId` / `uid` / `iCalUID`

Ts normalisation accepts epoch seconds, epoch millis (Gmail
`internalDate`), RFC-3339 strings, and bare `YYYY-MM-DD` dates.

Test coverage:
- 9 unit tests in composio.rs: Gmail + Calendar happy paths, calendar
  event_updated idempotency on same id, unknown/non-composio triggers
  are no-ops, normalize_* reject payloads missing id or text, ts
  parser accepts millis/seconds/RFC-3339/date.
- tests/composio_ingest_e2e.rs: publishes a real
  `DomainEvent::ComposioTriggerReceived` through the global event bus,
  asserts the bridge wrote an item visible via
  `openhuman.life_capture_search` with `source=gmail`, ts normalised
  from millis, and a re-fire of the same messageId does not inflate
  `total_items`.

Verification:
- cargo check --manifest-path Cargo.toml           ✓
- cargo test --lib life_capture                    ✓ (36 passed)
- cargo test --test composio_ingest_e2e --no-run   ✓

Out of scope (per v1 plan): iMessage scanner, chronicle, people/,
eventkit/, skills, telegram, slack, notion, onboarding UI.
@jwalin-shah
jwalin-shah force-pushed the feat/a2-composio-bridge branch from 35206ca to 999231d Compare April 25, 2026 23:13
@jwalin-shah
jwalin-shah marked this pull request as ready for review April 25, 2026 23:23
@jwalin-shah
jwalin-shah requested a review from a team April 25, 2026 23:24

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/core/all.rs (1)

252-311: ⚠️ Potential issue | 🟡 Minor

Add namespace_description entries for life_capture and curated_memory.

namespace_description powers the CLI help/--help output for each namespace. The two new namespaces will return None here, leaving their CLI entries un-described while every other namespace registered above has a one-liner. Cheap to add now and avoids drift in the catalogue users see.

📝 Proposed additions
         "learning" => Some(
             "User context enrichment — LinkedIn profile scraping and onboarding intelligence.",
         ),
+        "life_capture" => Some(
+            "Local personal index — search and stats over captured life data (Gmail, Calendar, iMessage, …).",
+        ),
+        "curated_memory" => Some(
+            "Agent-writable curated memory (MEMORY.md + USER.md) used for stable prompt snapshots.",
+        ),
         "notification" => Some(
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/all.rs` around lines 252 - 311, Add two match arms to the
namespace_description function to return descriptions for the new namespaces:
add "life_capture" => Some("Capture of user life events, media, and context for
personal history and recall.") and "curated_memory" => Some("Curated, reviewed
memory entries for long-term persistence and summarized retrieval."); place
these entries alongside the other namespace string arms in namespace_description
so CLI help shows one-line descriptions for both new namespaces.
app/src-tauri/src/imessage_scanner/mod.rs (1)

1-6: ⚠️ Potential issue | 🟡 Minor

Stale module docstring — references the old RPC method.

The header comment still claims this scanner emits openhuman.memory_doc_ingest, but the actual ingest call at line 421 now targets openhuman.life_capture_ingest. Update so future readers (and grep) land on the right method.

📝 Proposed fix
 //! iMessage local-database scanner.
 //!
 //! Reads `~/Library/Messages/chat.db` on macOS (read-only) and emits one
-//! `openhuman.memory_doc_ingest` JSON-RPC call per `(chat_identifier, day)`
+//! `openhuman.life_capture_ingest` JSON-RPC call per `(chat_identifier, day)`
 //! group — matching the convention codified in
 //! `docs/webview-integration-playbook.md` and used by the WhatsApp scanner.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src-tauri/src/imessage_scanner/mod.rs` around lines 1 - 6, Update the
module-level docstring at the top of imessage_scanner::mod.rs to reflect the
current RPC method name: replace references to `openhuman.memory_doc_ingest`
with `openhuman.life_capture_ingest` so the comment matches the actual call made
later in this module (see the ingest call in the code that invokes
`openhuman.life_capture_ingest`). Ensure the rest of the descriptive text (about
chat.db grouping and the referenced playbook) remains intact.
scripts/install.sh (1)

277-304: ⚠️ Potential issue | 🔴 Critical

Fix the regex patterns — doubled backslashes break macOS asset matching.

Inside the single-quoted heredoc <<'PY', Python receives the regex patterns with literal doubled backslashes. In a raw string, r"\\.dmg$" becomes the regex "literal backslash + any char + dmg + end", which cannot match real filenames like OpenHuman_0.1.0_aarch64.dmg.

Verification confirms all four patterns fail to match:

  • r"aarch64.*\\.app\\.tar\\.gz$" → no match
  • r"aarch64\\.dmg$" → no match
  • r"(x86_64-apple-darwin|x64).*\\.app\\.tar\\.gz$" → no match
  • r"x64\\.dmg$" → no match

This breaks the GitHub-Release-API fallback silently (masked as "no compatible asset found"). Replace each doubled backslash with a single backslash: r"aarch64\.dmg$", r"aarch64.*\.app\.tar\.gz$", r"(x86_64-apple-darwin|x64).*\.app\.tar\.gz$", r"x64\.dmg$".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/install.sh` around lines 277 - 304, The regex patterns used to select
macOS assets (e.g., r"aarch64.*\\.app\\.tar\\.gz$", r"aarch64\\.dmg$",
r"(x86_64-apple-darwin|x64).*\\.app\\.tar\\.gz$", r"x64\\.dmg$") are written
with doubled backslashes inside the single-quoted heredoc (<<'PY'), causing them
to look for literal backslashes and fail to match filenames; update those
patterns to use single backslashes (e.g., r"aarch64\.dmg$",
r"aarch64.*\.app\.tar\.gz$", r"(x86_64-apple-darwin|x64).*\.app\.tar\.gz$",
r"x64\.dmg$") so the loops that iterate over names and set chosen (and the
subsequent assets loop that compares asset.get("name") to chosen) correctly find
macOS assets.
🧹 Nitpick comments (18)
src/openhuman/agent/agents/integrations_agent/prompt.rs (1)

171-171: LGTM — required compile-fix for the updated PromptContext shape.

Side note: with seven agent prompt-test files now manually spelling out every PromptContext field, the duplication is approaching the point where a PromptContext::test_default() builder (or a test-only Default impl behind #[cfg(test)]) would make future field additions a one-line change instead of seven. Not blocking — flagging for a future cleanup pass.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/agent/agents/integrations_agent/prompt.rs` at line 171, The
compile error is due to the updated PromptContext shape missing the
curated_snapshot field; update the construction of PromptContext in
agents/integrations_agent/prompt.rs to include curated_snapshot: None (matching
the new struct field) so the struct literal compiles, and consider adding a
PromptContext::test_default() or a test-only Default impl for PromptContext to
reduce duplication across the seven agent prompt-test files in a future cleanup.
src/openhuman/tools/ops.rs (1)

122-126: Add a regression-guard test for life_capture_search registration.

The new tool is unconditionally registered (matching the comment that surfaces a structured error at call time if life_capture::runtime isn't initialised yet), but there's no test pinning that contract. Every other always-on tool added to this list (spawn_subagent, complete_onboarding, current_time, curl, …) ships with a matching assert!(names.contains(&"…")) regression guard; without one here, a future refactor that gates registration behind a feature/flag could silently strip the tool from the agent's tool list and slip through CI.

♻️ Proposed test (drop next to the other regression guards)
#[test]
fn all_tools_always_registers_life_capture_search() {
    // Regression guard: `life_capture_search` is always registered.
    // Surfacing a structured error from runtime::get_index/get_embedder
    // is the intended behavior when life-capture core hasn't initialised
    // — the tool itself must still appear in the registry.
    let tmp = TempDir::new().unwrap();
    let security = Arc::new(SecurityPolicy::default());
    let mem_cfg = MemoryConfig {
        backend: "markdown".into(),
        ..MemoryConfig::default()
    };
    let mem: Arc<dyn Memory> =
        Arc::from(crate::openhuman::memory::create_memory(&mem_cfg, tmp.path()).unwrap());
    let browser = BrowserConfig::default();
    let http = crate::openhuman::config::HttpRequestConfig::default();
    let cfg = test_config(&tmp);

    let tools = all_tools(
        Arc::new(Config::default()),
        &security,
        mem,
        &browser,
        &http,
        tmp.path(),
        &HashMap::new(),
        &cfg,
    );
    let names: Vec<&str> = tools.iter().map(|t| t.name()).collect();
    assert!(
        names.contains(&"life_capture_search"),
        "life_capture_search must always be registered; got: {names:?}"
    );
}

As per coding guidelines: "Rust code must be unit-tested for new/changed behavior before stacking features".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/tools/ops.rs` around lines 122 - 126, Add a regression-guard
unit test that ensures life_capture_search is always registered by calling
all_tools and asserting the returned tool names contain "life_capture_search";
mirror the existing pattern used for other always-on tools (create a TempDir,
SecurityPolicy, Memory via create_memory, BrowserConfig, HttpRequestConfig,
test_config, call all_tools, collect names with .iter().map(|t| t.name()), and
assert!(names.contains(&"life_capture_search"))). Place this test alongside the
other registration guards (next to tests that assert presence of
spawn_subagent/complete_onboarding/current_time/etc.) so future refactors that
accidentally gate or remove LifeCaptureSearchTool::new() will fail CI.
src/openhuman/life_capture/redact.rs (1)

8-17: Optional: anchor PHONE with word boundaries to reduce over-matching in opaque IDs.

PHONE has no \b at start/end, so digit substrings inside identifiers (tracking numbers, hex-adjacent IDs, etc.) can be partially rewritten as <PHONE>. The other three regexes use \b and don't have this property. Since this redaction runs upstream of embedding + FTS indexing, an over-match merely costs you recall on those tokens — not a correctness issue — but tightening it is cheap:

♻️ Suggested boundary tightening
 static PHONE: Lazy<Regex> = Lazy::new(|| {
     Regex::new(
         r"(?x)
+        \b
         (?:\+?\d{1,3}[\s\-.])?       # optional country code, e.g. '+1-' or '1.'
         (?:\(\d{2,4}\)|\d{2,4})[\s\-.]?
         \d{3}[\s\-.]?\d{3,4}
+        \b
     ",
     )
     .unwrap()
 });

Worth adding a regression test that a long opaque ID like "order-1234567890123" is left untouched (currently the CC regex would also catch this — same caveat applies but it's a separate, well-known best-effort tradeoff).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/redact.rs` around lines 8 - 17, The PHONE regex
(static PHONE: Lazy<Regex>) lacks word boundaries and can over-match digit
substrings inside opaque IDs; update the regex to add \b anchors at the start
and end of the pattern (i.e., wrap the whole pattern with \b ... \b or
equivalent) so it only matches standalone phone-like tokens, and add a unit test
asserting strings like "order-1234567890123" are not redacted (and other
positive tests to ensure real phone numbers still redact).
src/openhuman/life_capture/migrations/0001_init.sql (1)

9-9: Clarify or wire up text_keyword.

The column is declared with a comment promising a "raw values projection for FTS5/BM25 (Onyx pattern)", but items_fts indexes subject, text and all three triggers ignore text_keyword. Future readers will reasonably assume it's indexed. Either:

  1. Index it in items_fts (and update the AI/AD/AU triggers accordingly) so keyword search actually uses the unprocessed projection, or
  2. Drop the column from this migration and re-introduce it in the migration that first uses it, or
  3. Rephrase the comment to make clear it's a placeholder reserved for a follow-up migration.

Option 3 is the lowest-risk if you want to keep the column shape stable for future readers/writers without touching trigger logic in this PR.

Also applies to: 30-36

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/migrations/0001_init.sql` at line 9, The column
text_keyword is declared but not used by items_fts or the AI/AD/AU triggers,
which is misleading; update this migration to either wire it up or make its
intent explicit—prefer the low-risk option: keep the column but change its
inline comment to state it's a reserved placeholder for a future migration
(e.g., "reserved for raw values projection for FTS5/BM25; indexed and wired up
in a follow-up migration"), and leave items_fts and the triggers (AI/AD/AU)
unchanged so behavior stays stable.
src/openhuman/agent/harness/session/types.rs (1)

135-136: Use the in-scope Arc for consistency.

Arc is already imported at line 19 and used unqualified throughout this file (e.g., line 27 Arc<dyn Provider>, line 142 Arc<dyn ... PayloadSummarizer>). The new field could drop the std::sync:: prefix to match.

♻️ Proposed tweak
-    pub(super) curated_snapshot:
-        Option<std::sync::Arc<crate::openhuman::curated_memory::MemorySnapshot>>,
+    pub(super) curated_snapshot: Option<Arc<crate::openhuman::curated_memory::MemorySnapshot>>,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/agent/harness/session/types.rs` around lines 135 - 136, The
field curated_snapshot is using the fully qualified std::sync::Arc type; change
it to use the in-scope Arc to match the rest of the file (e.g., other usages
like Arc<dyn Provider> and Arc<dyn PayloadSummarizer>). Locate the declaration
of curated_snapshot (type
Option<std::sync::Arc<crate::openhuman::curated_memory::MemorySnapshot>>) and
replace std::sync::Arc with the unqualified Arc so the field reads
Option<Arc<crate::openhuman::curated_memory::MemorySnapshot>>>, keeping the same
crate path for MemorySnapshot.
src/openhuman/life_capture/quote_strip.rs (1)

4-7: Edge case: wrapped On … wrote: headers won't match.

Gmail/Apple Mail occasionally wrap the citation header across two lines when the sender name + email is long (On Mon, Apr 21, 2026 at 9:14 AM, Some Long Name\n<long.address@example.com> wrote:). Since . doesn't match newlines, the cut won't fire and the wrapped header + quoted block will leak through (the >-line filter catches the quotes, but the wrapped header stays). Not a release blocker — typical clients keep it on one line — but worth a (?s) variant or a permissive [\s\S]{1,400} if you see this in real Gmail payloads during A2 e2e.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/quote_strip.rs` around lines 4 - 7, The
ON_DATE_WROTE regex currently uses .{1,200} which won't match when the "On …
wrote:" header is wrapped across lines; update the pattern in the Lazy::new
closure (where Regex::new is called for ON_DATE_WROTE) to allow newlines in that
span (either add the DOTALL flag via (?s) or replace .{1,200} with a permissive
[\s\S]{1,400} or similar) so wrapped headers like "On ...\n<email> wrote:" are
matched; keep the word boundary and trailing wrote:\s*$ anchors intact and
adjust the max length as needed.
app/src-tauri/src/imessage_scanner/mod.rs (1)

413-452: Note: ts is UTC midnight of a local-tz day.

seconds_to_ymd (line 276) renders the day in local timezone, but parse_day_to_unix parses it back as UTC midnight. For non-UTC users this puts ts up to ~24h off "actual local midnight" — fine for dedupe (the stable key is external_id, not ts), but the comment on line 413 reads as if ts is the precise start-of-day in UTC. Either tighten the comment to "stable anchor derived from the local-tz day" or compute the local-tz midnight here so semantic ranking by ts matches the user's calendar.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src-tauri/src/imessage_scanner/mod.rs` around lines 413 - 452, The
comment claims ts is "UTC midnight of a local-tz day" but seconds_to_ymd renders
day in the local timezone while parse_day_to_unix currently produces UTC
midnight, causing up-to ~24h mismatch; either update the comment to say ts is a
stable anchor derived from the local-tz day, or change parse_day_to_unix to
compute the local-tz midnight for the provided YYYY-MM-DD string and return its
UTC unix-seconds instead (use chrono::Local or convert
NaiveDate.and_hms_opt(0,0,0) with Local::from_local_datetime and then to_utc
timestamp). Update the code in parse_day_to_unix (and keep seconds_to_ymd
behavior) so ts semantics match the comment, or leave parse_day_to_unix as-is
and tighten the comment to refer to a "stable anchor derived from the local-tz
day" mentioning parse_day_to_unix, seconds_to_ymd, and the ts variable.
tests/composio_ingest_e2e.rs (1)

67-72: Silent error-swallowing on init_index/init_embedder makes this test fragile if it ever stops being #[ignore].

Both calls are wrapped in let _ = …, so if the runtime singletons are already populated by an earlier test in the same binary the bridge is wired to the local idx while the JSON-RPC path (openhuman.life_capture_search / _get_stats) reads from the previously-installed runtime — the assertions then fail with no obvious diagnostic. Today this is masked by #[ignore], but if anyone removes that gate later this becomes a confusing flake.

Either log/assert on the init result, or document the single-test invariant inline:

📝 Surface init mismatches loudly
-    let _ = runtime::init_index(Arc::clone(&idx)).await;
+    if let Err(e) = runtime::init_index(Arc::clone(&idx)).await {
+        eprintln!("[composio_e2e] runtime::init_index already set ({e}) — this test must run alone");
+    }
     let embedder: Arc<dyn Embedder> = Arc::new(DeterministicEmbedder { dim: 1536 });
-    let _ = runtime::init_embedder(Arc::clone(&embedder)).await;
+    if let Err(e) = runtime::init_embedder(Arc::clone(&embedder)).await {
+        eprintln!("[composio_e2e] runtime::init_embedder already set ({e})");
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/composio_ingest_e2e.rs` around lines 67 - 72, The test currently
silences errors from runtime::init_index and runtime::init_embedder which can
hide mismatched singleton state; change the calls to check and propagate
failures instead of using `let _ = ...` — call
runtime::init_index(Arc::clone(&idx)).await and
runtime::init_embedder(Arc::clone(&embedder)).await and assert or unwrap their
Results (or log and panic) so any attempt to reinitialize with a different
PersonalIndex (opened via PersonalIndex::open) or Embedder
(DeterministicEmbedder) fails loudly before calling
register_life_capture_composio_bridge; alternatively add an explicit inline
comment documenting the single-test invariant if you must keep the silent
behavior.
src/openhuman/life_capture/runtime.rs (1)

26-38: init_index / init_embedder are async fn but never .await — drop the async.

OnceCell::set is synchronous. Marking these functions async adds spurious async-coloring to startup with zero benefit. All six call sites (in src/core/jsonrpc.rs and test files) use .await and would need it removed:

♻️ Proposed change
-pub async fn init_index(idx: Arc<PersonalIndex>) -> Result<(), &'static str> {
+pub fn init_index(idx: Arc<PersonalIndex>) -> Result<(), &'static str> {
     INDEX
         .set(idx)
         .map_err(|_| "life_capture index already initialised")
 }

-pub async fn init_embedder(embedder: Arc<dyn Embedder>) -> Result<(), &'static str> {
+pub fn init_embedder(embedder: Arc<dyn Embedder>) -> Result<(), &'static str> {
     EMBEDDER
         .set(embedder)
         .map_err(|_| "life_capture embedder already initialised")
 }

Update call sites to remove .await:

  • src/core/jsonrpc.rs (lines 940, 973)
  • tests/imessage_ingest_e2e.rs (lines 73, 75)
  • tests/composio_ingest_e2e.rs (lines 68, 70)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/runtime.rs` around lines 26 - 38, The functions
init_index and init_embedder are declared async but only call synchronous
OnceCell::set (INDEX.set and EMBEDDER.set); remove the async from both
signatures so they are regular synchronous functions returning Result<(),
&'static str>, and update all call sites that currently do `.await` on
init_index/init_embedder to call them without awaiting (i.e., remove the
`.await`), leaving their return type and error handling unchanged.
src/openhuman/life_capture/embedder.rs (2)

97-99: Hard-coded dim() = 1536 couples this struct to a single model family.

text-embedding-3-small is 1536-dim, but text-embedding-3-large is 3072 and other providers vary. Since model is already a configurable field, a future config swap will fail the dim-check at lines 87‑92 with a confusing error. Consider taking the dimension via HostedEmbedder::new(...) or auto-detecting from the first response.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/embedder.rs` around lines 97 - 99, The dim()
method currently returns a hard-coded 1536 which ties the embedder to one model;
change HostedEmbedder so dimension is configurable (e.g., add a dim field set
via HostedEmbedder::new(...)) or implement auto-detection by calling the
embedding API once and storing the returned vector length, then have dim()
return that stored value; update any construction sites that create
HostedEmbedder to supply/derive the correct dimension and remove the fixed 1536
constant so model swaps won’t break the runtime check in the embedding
validation logic.

54-95: Add logging around the external embeddings call.

This is a remote HTTP dependency on the request-serving path. Consider adding tracing::debug! with [life_capture:embedder] prefix at entry (input count, model, base_url host only — never api_key), and tracing::warn! on error_for_status/JSON parse failures with the response status. As per coding guidelines: "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls, retries/timeouts" and "Never log secrets, raw JWTs, API keys".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/embedder.rs` around lines 54 - 95, The embed_batch
function makes an external HTTP call without development logging; add a
tracing::debug! at the start of embed_batch that logs the input count,
self.model, and only the host portion of self.base_url (do NOT log self.api_key
or any secrets) with a prefix like "[life_capture:embedder]"; then wrap the HTTP
response handling to emit tracing::warn! (also prefixed) when error_for_status()
or the subsequent JSON deserialization fails, including the HTTP status code and
any non-sensitive response context (but not body or api_key), and include a
final tracing::debug! on success (e.g., returned embeddings count and dim)
before returning.
src/openhuman/curated_memory/store.rs (2)

58-60: Drop a more informative error like add does.

add reports "char limit {} exceeded" (line 42) but replace just says "char limit". Trivial consistency win for callers reading the error.

♻️ Tweak
         if next.chars().count() > self.char_limit {
-            return Err(std::io::Error::new(std::io::ErrorKind::Other, "char limit"));
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                format!("char limit {} exceeded", self.char_limit),
+            ));
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/curated_memory/store.rs` around lines 58 - 60, The replace
branch returns a non-descriptive error string "char limit" when
next.chars().count() > self.char_limit; make it consistent with add by returning
an error that includes the limit value and context (e.g., "char limit {}
exceeded") so callers see the threshold—update the error created in replace to
format the message with self.char_limit, matching the add implementation.

98-102: with_extension("md.tmp") is fragile if MemoryFile::filename() ever stops being .md.

Path::with_extension replaces the existing extension. Today MemoryFile::filename() always ends in .md so memory.md → memory.md.tmp, but if a future variant returns notes.json, this would write to notes.md.tmp and silently lose the .json. Appending instead of replacing is more robust, e.g. path.with_file_name(format!("{}.tmp", path.file_name().unwrap().to_string_lossy())). Minor — current callers are fine.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/curated_memory/store.rs` around lines 98 - 102, The
atomic_write function uses path.with_extension("md.tmp") which replaces the
existing extension and will break if MemoryFile::filename() stops using .md;
change it to append a .tmp suffix to the entire filename instead (use
path.file_name() + ".tmp" and call path.with_file_name(...)) so e.g.
atomic_write builds tmp via something like path.with_file_name(format!("{}.tmp",
path.file_name().unwrap().to_string_lossy())); ensure you handle the None case
for file_name (return an io::Error) and keep the rest of atomic_write
(tokio::fs::write then tokio::fs::rename) unchanged.
src/openhuman/life_capture/rpc.rs (2)

76-82: Constant duplicated with index.rs — extract to types.rs to keep dim authoritative.

INDEX_VECTOR_DIM = 1536 here mirrors dim() = 1536 in embedder.rs and the vec0 schema width in index.rs. If a future migration changes the column width, three places have to move in lockstep. Promoting this to a pub const VECTOR_DIM: usize = 1536; in types.rs (or index.rs) and importing it into both call sites would keep things honest.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/rpc.rs` around lines 76 - 82, The duplicate
hard-coded vector dimension (INDEX_VECTOR_DIM = 1536) used in rpc.rs should be
centralized: create a public constant pub const VECTOR_DIM: usize = 1536 in
types.rs (or an existing index-related module), remove the local
INDEX_VECTOR_DIM, and import VECTOR_DIM into rpc.rs to use in the embedder.dim()
check (replace INDEX_VECTOR_DIM with VECTOR_DIM). Update any other modules
(e.g., where vec0 schema width or embedder.rs currently assume 1536) to
reference the same VECTOR_DIM to keep the dimension authoritative across
INDEX_VECTOR_DIM, embedder.dim(), and index schema usages.

142-170: Item is committed before the embedding succeeds — retry semantics are okay, but document/log the partial state.

If embed_batch or upsert_vector fails after upsert returns, the row exists in items/items_fts with no vector. Keyword search will still surface it; vector search won't. The next re-ingest by the same (source, external_id) will re-embed and patch it, so this is self-healing — but only if the caller retries.

Two suggestions:

  1. Log a tracing::warn! (with item_id/source/external_id) on the embed/upsert_vector error paths so partial states are observable.
  2. Consider noting this contract in the rustdoc on handle_ingest so callers of the RPC know to retry on error to backfill the vector.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/rpc.rs` around lines 142 - 170, The upsert of the
Item (requested_id -> canonical_id) happens before embedding, so failures in
embed_batch or upsert_vector leave a row without a vector; update the
embed/upsert_vector error paths to emit a tracing::warn! that includes the item
id (canonical_id or requested_id), source and external_id so partial states are
observable, and ensure the warn is logged just before returning the error from
both the embed_batch .map_err and the upsert_vector .map_err locations;
additionally add a short rustdoc on the handle_ingest RPC function explaining
that callers should retry on error to allow a subsequent ingest to backfill the
missing vector.
src/openhuman/life_capture/ingest/composio.rs (2)

364-378: log::debug!/log::warn! here while the rest of the file uses tracing — pick one.

The bridge's hot path uses tracing::{debug,warn}! (lines 57‑112), but registration uses log::{debug,warn}!. Both routes ultimately reach the configured subscriber, but mixing facades makes filtering by target less predictable and adds a small papercut for anyone grepping with a single prefix. Prefer tracing to stay consistent with the rest of the bridge and the codebase's [domain] prefix convention.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/ingest/composio.rs` around lines 364 - 378,
Replace the mixed use of log macros with tracing in the bridge registration
path: where BRIDGE_HANDLE is checked and the bridge is created with
LifeCaptureComposioBridge::new and registered via subscribe_global, change the
log::debug! and log::warn! calls to tracing::debug! and tracing::warn! (or
import tracing::{debug, warn}) so the registration messages use the same tracing
facade/target as the rest of the file and maintain consistent filtering and
[domain] prefix behavior.

417-424: std::mem::forget(dir) leaks the temp dir for the process lifetime.

The intent is "keep the directory alive past the function return so the SQLite file at db remains valid". TempDir::into_path() expresses that explicitly (and lets the OS reclaim it on test-runner cleanup) without going through mem::forget. Lower priority since this is test-only.

♻️ Proposed tweak
-    async fn fresh_bridge() -> LifeCaptureComposioBridge {
-        let dir = tempdir().expect("tempdir");
-        let db = dir.path().join("lc.db");
-        std::mem::forget(dir);
-        let idx = Arc::new(PersonalIndex::open(&db).await.expect("open"));
+    async fn fresh_bridge() -> LifeCaptureComposioBridge {
+        let dir = tempdir().expect("tempdir").into_path();
+        let db = dir.join("lc.db");
+        let idx = Arc::new(PersonalIndex::open(&db).await.expect("open"));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/ingest/composio.rs` around lines 417 - 424, The
test helper fresh_bridge currently calls std::mem::forget(dir) which improperly
leaks the TempDir; replace that with TempDir::into_path() so the temp directory
is explicitly preserved for the lifetime of the test-runner cleanup. Concretely:
create the tempdir as before, call dir.into_path() (or assign its result to a
new PathBuf) to construct the db path used to open PersonalIndex, remove the
std::mem::forget call, and keep the rest (opening PersonalIndex, creating
Arc<TestEmbedder>, and calling LifeCaptureComposioBridge::new) unchanged.
src/openhuman/life_capture/schemas.rs (1)

226-272: Adapter logging — consider emitting trace/debug at entry/exit for each handler.

These adapters cross the JSON-RPC → domain boundary and currently log nothing on entry, exit, or failure paths. A tracing::debug! with a stable [life_capture:rpc] prefix and the controller name + key params (e.g. source, external_id for ingest; k and a redacted text.len() for search) would make request flows traceable without leaking PII (don't log text body or metadata). As per coding guidelines: "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/schemas.rs` around lines 226 - 272, Add
development-oriented trace/debug logs to the RPC adapters: in handle_get_stats,
handle_search, and handle_ingest emit tracing::debug! logs with a stable
"[life_capture:rpc]" prefix at function entry (and at successful exit) and on
error branches; log only non-PII controller identifiers — for handle_ingest
include source and external_id, for handle_search include k and text.len()
(redacted), and for handle_get_stats a simple entry/exit marker — do not log
text bodies or metadata. Place entry logs before calling
runtime::get_index()/runtime::get_embedder() and downstream RPC calls, place
exit logs after the to_json(...) result is produced, and include error context
when mapping runtime errors or serde errors to strings so failures are traceable
without leaking sensitive content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/install.sh`:
- Around line 369-371: The call to resolve_from_release_api is currently a bare
command under set -e so the script will exit on nonzero returns before
resolve_rc is captured; change the invocation to an errexit-safe form (e.g.,
call it inside an if condition like `if resolve_from_release_api; then ... fi`
or append `|| true` so the function can return a nonzero code without exiting),
then capture its exit code into resolve_rc and let the existing dry-run/DRY RUN
messaging branch run when resolve_rc != 0; update the invocation site around
resolve_from_release_api and the resolve_rc handling so the dry-run branch (the
code that prints "DRY RUN: no compatible artifact available yet") is reachable.

In `@src/core/jsonrpc.rs`:
- Around line 949-989: The current early return when OPENHUMAN_EMBEDDINGS_KEY /
OPENAI_API_KEY is missing prevents register_life_capture_composio_bridge from
being called, silently disabling Composio Gmail/Calendar ingest; instead, keep
the bridge wired regardless of an embedder by moving the call to
crate::openhuman::life_capture::ingest::register_life_capture_composio_bridge(Arc::clone(&idx),
embedder_optional) out of the early-return path (or create a no-op LocalEmbedder
placeholder and still construct/embed it) and change the log message around the
api_key check to clearly state that the embedder is not configured but the
Composio bridge remains registered (or will use a noop embedder) so ingest still
flows to life_capture even when embeddings are not configured.

In `@src/openhuman/life_capture/embedder.rs`:
- Around line 21-25: The current construction of the reqwest client assigns to
http using reqwest::Client::builder().build().unwrap_or_else(|_|
reqwest::Client::new()), which silently drops the configured timeouts if build()
fails; change this so the error is not swallowed — either propagate the build
error by returning anyhow::Result (update the enclosing constructor function to
return Result and bubble up the reqwest::Error) or replace unwrap_or_else with
expect() so the process fails loudly on startup; locate the http assignment
(reqwest::Client::builder(), .timeout(), .connect_timeout(), .build()) in
embedder.rs and update the surrounding function signature/return path
accordingly.

In `@src/openhuman/life_capture/ingest/composio.rs`:
- Around line 604-623: The test currently asserts a tautology and never calls
the bridge methods; replace the dead Stub and the local `name` tautology with an
actual bridge instance (use `fresh_bridge().await` or whichever async helper
constructs `LifeCaptureComposioBridge`) and call `name()` and `domains()` on
that instance, asserting `bridge.name() == "life_capture::composio_bridge"` and
that `bridge.domains()` contains the expected domain(s) (or equals the expected
slice), ensuring the test is async if needed and awaits `fresh_bridge()`; remove
the unused `Stub` impl and any unused imports.
- Around line 55-75: The Gmail/Calendar ingest flow currently passes raw text
from normalize_gmail/normalize_calendar through do_ingest → handle_ingest →
IndexWriter::upsert without redaction or quoted-reply stripping; update the
ingest path to run the extracted text (and any text-like metadata) through the
quote_strip and redact utilities before it reaches IndexWriter::upsert.
Concretely, amend ingest_gmail/ingest_calendar or centralize in
do_ingest/handle_ingest to call quote_strip::strip (or equivalent) first, then
redact::redact (or equivalent) on the resulting string(s), and pass the
sanitized subject/text/metadata onwards so IndexWriter::upsert never receives
raw unredacted content. Ensure you reference normalize_gmail/normalize_calendar
inputs and preserve timestamps/external_id while replacing text fields with the
sanitized versions.

In `@src/openhuman/life_capture/schemas.rs`:
- Around line 188-193: The schema declares the FieldSchema for "metadata" as
TypeSchema::Option(Box::new(TypeSchema::String)) but the request handler reads
it as a serde_json::Value (defaulting to an empty Object), so change the schema
to reflect the actual contract: update the FieldSchema for "metadata" to
TypeSchema::Option(Box::new(TypeSchema::Value)) or a concrete JSON object
variant so clients know to send JSON, and ensure the handler still uses the
serde_json::Value path; alternatively, if you prefer to keep TypeSchema::String,
modify the handler code that reads "metadata" to parse the incoming string into
serde_json::Value (and validate errors) instead of accepting arbitrary objects —
update whichever of FieldSchema::metadata or the handler's parsing logic you
choose so schema and handler agree.

In `@src/openhuman/tools/impl/life_capture/search.rs`:
- Around line 67-71: The code reads k from args into variable k (via
args.get("k").and_then(Value::as_u64).map_or(10, |v| v as usize)) but does not
enforce the documented/schema cap of 100; update the parsing so k is clamped to
the range [1,100] before calling handle_search (e.g., read the u64, convert
safely, then apply min/max/clamp) to ensure the value respect the
parameters_schema and prevents unbounded work by handle_search/hybrid_search.

In `@tests/fixtures/life_capture/corpus.json`:
- Around line 320-332: The negative set for the test case with id "q-src-01" is
missing an additional non-calendar Phoenix item; update the object for q-src-01
to add "phoenix-weather-noise-01" to the must_not_contain array (and keep
must_not_contain_in_top as-is) so the query will correctly assert that
calendar-only results exclude that Gmail item once the pending gate is removed.

In `@tests/imessage_ingest_e2e.rs`:
- Around line 159-167: The assertion that stats1["total_items"] == 1 is brittle
due to shared process-global PersonalIndex; before performing the first ingest,
fetch a baseline with post_rpc(&base, "openhuman.life_capture_get_stats",
json!({})) (reuse rpc_result_body to parse) and record baseline_total =
stats_baseline["total_items"], then after ingest compute delta =
stats1["total_items"] - baseline_total and assert delta == 1; update the test to
use these symbols (env_stats_baseline, stats_baseline, baseline_total,
env_stats1, stats1) so the test asserts a delta rather than an absolute total.

---

Outside diff comments:
In `@app/src-tauri/src/imessage_scanner/mod.rs`:
- Around line 1-6: Update the module-level docstring at the top of
imessage_scanner::mod.rs to reflect the current RPC method name: replace
references to `openhuman.memory_doc_ingest` with `openhuman.life_capture_ingest`
so the comment matches the actual call made later in this module (see the ingest
call in the code that invokes `openhuman.life_capture_ingest`). Ensure the rest
of the descriptive text (about chat.db grouping and the referenced playbook)
remains intact.

In `@scripts/install.sh`:
- Around line 277-304: The regex patterns used to select macOS assets (e.g.,
r"aarch64.*\\.app\\.tar\\.gz$", r"aarch64\\.dmg$",
r"(x86_64-apple-darwin|x64).*\\.app\\.tar\\.gz$", r"x64\\.dmg$") are written
with doubled backslashes inside the single-quoted heredoc (<<'PY'), causing them
to look for literal backslashes and fail to match filenames; update those
patterns to use single backslashes (e.g., r"aarch64\.dmg$",
r"aarch64.*\.app\.tar\.gz$", r"(x86_64-apple-darwin|x64).*\.app\.tar\.gz$",
r"x64\.dmg$") so the loops that iterate over names and set chosen (and the
subsequent assets loop that compares asset.get("name") to chosen) correctly find
macOS assets.

In `@src/core/all.rs`:
- Around line 252-311: Add two match arms to the namespace_description function
to return descriptions for the new namespaces: add "life_capture" =>
Some("Capture of user life events, media, and context for personal history and
recall.") and "curated_memory" => Some("Curated, reviewed memory entries for
long-term persistence and summarized retrieval."); place these entries alongside
the other namespace string arms in namespace_description so CLI help shows
one-line descriptions for both new namespaces.

---

Nitpick comments:
In `@app/src-tauri/src/imessage_scanner/mod.rs`:
- Around line 413-452: The comment claims ts is "UTC midnight of a local-tz day"
but seconds_to_ymd renders day in the local timezone while parse_day_to_unix
currently produces UTC midnight, causing up-to ~24h mismatch; either update the
comment to say ts is a stable anchor derived from the local-tz day, or change
parse_day_to_unix to compute the local-tz midnight for the provided YYYY-MM-DD
string and return its UTC unix-seconds instead (use chrono::Local or convert
NaiveDate.and_hms_opt(0,0,0) with Local::from_local_datetime and then to_utc
timestamp). Update the code in parse_day_to_unix (and keep seconds_to_ymd
behavior) so ts semantics match the comment, or leave parse_day_to_unix as-is
and tighten the comment to refer to a "stable anchor derived from the local-tz
day" mentioning parse_day_to_unix, seconds_to_ymd, and the ts variable.

In `@src/openhuman/agent/agents/integrations_agent/prompt.rs`:
- Line 171: The compile error is due to the updated PromptContext shape missing
the curated_snapshot field; update the construction of PromptContext in
agents/integrations_agent/prompt.rs to include curated_snapshot: None (matching
the new struct field) so the struct literal compiles, and consider adding a
PromptContext::test_default() or a test-only Default impl for PromptContext to
reduce duplication across the seven agent prompt-test files in a future cleanup.

In `@src/openhuman/agent/harness/session/types.rs`:
- Around line 135-136: The field curated_snapshot is using the fully qualified
std::sync::Arc type; change it to use the in-scope Arc to match the rest of the
file (e.g., other usages like Arc<dyn Provider> and Arc<dyn PayloadSummarizer>).
Locate the declaration of curated_snapshot (type
Option<std::sync::Arc<crate::openhuman::curated_memory::MemorySnapshot>>) and
replace std::sync::Arc with the unqualified Arc so the field reads
Option<Arc<crate::openhuman::curated_memory::MemorySnapshot>>>, keeping the same
crate path for MemorySnapshot.

In `@src/openhuman/curated_memory/store.rs`:
- Around line 58-60: The replace branch returns a non-descriptive error string
"char limit" when next.chars().count() > self.char_limit; make it consistent
with add by returning an error that includes the limit value and context (e.g.,
"char limit {} exceeded") so callers see the threshold—update the error created
in replace to format the message with self.char_limit, matching the add
implementation.
- Around line 98-102: The atomic_write function uses
path.with_extension("md.tmp") which replaces the existing extension and will
break if MemoryFile::filename() stops using .md; change it to append a .tmp
suffix to the entire filename instead (use path.file_name() + ".tmp" and call
path.with_file_name(...)) so e.g. atomic_write builds tmp via something like
path.with_file_name(format!("{}.tmp",
path.file_name().unwrap().to_string_lossy())); ensure you handle the None case
for file_name (return an io::Error) and keep the rest of atomic_write
(tokio::fs::write then tokio::fs::rename) unchanged.

In `@src/openhuman/life_capture/embedder.rs`:
- Around line 97-99: The dim() method currently returns a hard-coded 1536 which
ties the embedder to one model; change HostedEmbedder so dimension is
configurable (e.g., add a dim field set via HostedEmbedder::new(...)) or
implement auto-detection by calling the embedding API once and storing the
returned vector length, then have dim() return that stored value; update any
construction sites that create HostedEmbedder to supply/derive the correct
dimension and remove the fixed 1536 constant so model swaps won’t break the
runtime check in the embedding validation logic.
- Around line 54-95: The embed_batch function makes an external HTTP call
without development logging; add a tracing::debug! at the start of embed_batch
that logs the input count, self.model, and only the host portion of
self.base_url (do NOT log self.api_key or any secrets) with a prefix like
"[life_capture:embedder]"; then wrap the HTTP response handling to emit
tracing::warn! (also prefixed) when error_for_status() or the subsequent JSON
deserialization fails, including the HTTP status code and any non-sensitive
response context (but not body or api_key), and include a final tracing::debug!
on success (e.g., returned embeddings count and dim) before returning.

In `@src/openhuman/life_capture/ingest/composio.rs`:
- Around line 364-378: Replace the mixed use of log macros with tracing in the
bridge registration path: where BRIDGE_HANDLE is checked and the bridge is
created with LifeCaptureComposioBridge::new and registered via subscribe_global,
change the log::debug! and log::warn! calls to tracing::debug! and
tracing::warn! (or import tracing::{debug, warn}) so the registration messages
use the same tracing facade/target as the rest of the file and maintain
consistent filtering and [domain] prefix behavior.
- Around line 417-424: The test helper fresh_bridge currently calls
std::mem::forget(dir) which improperly leaks the TempDir; replace that with
TempDir::into_path() so the temp directory is explicitly preserved for the
lifetime of the test-runner cleanup. Concretely: create the tempdir as before,
call dir.into_path() (or assign its result to a new PathBuf) to construct the db
path used to open PersonalIndex, remove the std::mem::forget call, and keep the
rest (opening PersonalIndex, creating Arc<TestEmbedder>, and calling
LifeCaptureComposioBridge::new) unchanged.

In `@src/openhuman/life_capture/migrations/0001_init.sql`:
- Line 9: The column text_keyword is declared but not used by items_fts or the
AI/AD/AU triggers, which is misleading; update this migration to either wire it
up or make its intent explicit—prefer the low-risk option: keep the column but
change its inline comment to state it's a reserved placeholder for a future
migration (e.g., "reserved for raw values projection for FTS5/BM25; indexed and
wired up in a follow-up migration"), and leave items_fts and the triggers
(AI/AD/AU) unchanged so behavior stays stable.

In `@src/openhuman/life_capture/quote_strip.rs`:
- Around line 4-7: The ON_DATE_WROTE regex currently uses .{1,200} which won't
match when the "On … wrote:" header is wrapped across lines; update the pattern
in the Lazy::new closure (where Regex::new is called for ON_DATE_WROTE) to allow
newlines in that span (either add the DOTALL flag via (?s) or replace .{1,200}
with a permissive [\s\S]{1,400} or similar) so wrapped headers like "On
...\n<email> wrote:" are matched; keep the word boundary and trailing wrote:\s*$
anchors intact and adjust the max length as needed.

In `@src/openhuman/life_capture/redact.rs`:
- Around line 8-17: The PHONE regex (static PHONE: Lazy<Regex>) lacks word
boundaries and can over-match digit substrings inside opaque IDs; update the
regex to add \b anchors at the start and end of the pattern (i.e., wrap the
whole pattern with \b ... \b or equivalent) so it only matches standalone
phone-like tokens, and add a unit test asserting strings like
"order-1234567890123" are not redacted (and other positive tests to ensure real
phone numbers still redact).

In `@src/openhuman/life_capture/rpc.rs`:
- Around line 76-82: The duplicate hard-coded vector dimension (INDEX_VECTOR_DIM
= 1536) used in rpc.rs should be centralized: create a public constant pub const
VECTOR_DIM: usize = 1536 in types.rs (or an existing index-related module),
remove the local INDEX_VECTOR_DIM, and import VECTOR_DIM into rpc.rs to use in
the embedder.dim() check (replace INDEX_VECTOR_DIM with VECTOR_DIM). Update any
other modules (e.g., where vec0 schema width or embedder.rs currently assume
1536) to reference the same VECTOR_DIM to keep the dimension authoritative
across INDEX_VECTOR_DIM, embedder.dim(), and index schema usages.
- Around line 142-170: The upsert of the Item (requested_id -> canonical_id)
happens before embedding, so failures in embed_batch or upsert_vector leave a
row without a vector; update the embed/upsert_vector error paths to emit a
tracing::warn! that includes the item id (canonical_id or requested_id), source
and external_id so partial states are observable, and ensure the warn is logged
just before returning the error from both the embed_batch .map_err and the
upsert_vector .map_err locations; additionally add a short rustdoc on the
handle_ingest RPC function explaining that callers should retry on error to
allow a subsequent ingest to backfill the missing vector.

In `@src/openhuman/life_capture/runtime.rs`:
- Around line 26-38: The functions init_index and init_embedder are declared
async but only call synchronous OnceCell::set (INDEX.set and EMBEDDER.set);
remove the async from both signatures so they are regular synchronous functions
returning Result<(), &'static str>, and update all call sites that currently do
`.await` on init_index/init_embedder to call them without awaiting (i.e., remove
the `.await`), leaving their return type and error handling unchanged.

In `@src/openhuman/life_capture/schemas.rs`:
- Around line 226-272: Add development-oriented trace/debug logs to the RPC
adapters: in handle_get_stats, handle_search, and handle_ingest emit
tracing::debug! logs with a stable "[life_capture:rpc]" prefix at function entry
(and at successful exit) and on error branches; log only non-PII controller
identifiers — for handle_ingest include source and external_id, for
handle_search include k and text.len() (redacted), and for handle_get_stats a
simple entry/exit marker — do not log text bodies or metadata. Place entry logs
before calling runtime::get_index()/runtime::get_embedder() and downstream RPC
calls, place exit logs after the to_json(...) result is produced, and include
error context when mapping runtime errors or serde errors to strings so failures
are traceable without leaking sensitive content.

In `@src/openhuman/tools/ops.rs`:
- Around line 122-126: Add a regression-guard unit test that ensures
life_capture_search is always registered by calling all_tools and asserting the
returned tool names contain "life_capture_search"; mirror the existing pattern
used for other always-on tools (create a TempDir, SecurityPolicy, Memory via
create_memory, BrowserConfig, HttpRequestConfig, test_config, call all_tools,
collect names with .iter().map(|t| t.name()), and
assert!(names.contains(&"life_capture_search"))). Place this test alongside the
other registration guards (next to tests that assert presence of
spawn_subagent/complete_onboarding/current_time/etc.) so future refactors that
accidentally gate or remove LifeCaptureSearchTool::new() will fail CI.

In `@tests/composio_ingest_e2e.rs`:
- Around line 67-72: The test currently silences errors from runtime::init_index
and runtime::init_embedder which can hide mismatched singleton state; change the
calls to check and propagate failures instead of using `let _ = ...` — call
runtime::init_index(Arc::clone(&idx)).await and
runtime::init_embedder(Arc::clone(&embedder)).await and assert or unwrap their
Results (or log and panic) so any attempt to reinitialize with a different
PersonalIndex (opened via PersonalIndex::open) or Embedder
(DeterministicEmbedder) fails loudly before calling
register_life_capture_composio_bridge; alternatively add an explicit inline
comment documenting the single-test invariant if you must keep the silent
behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5d56ec5b-86da-4d95-83da-54582938622d

📥 Commits

Reviewing files that changed from the base of the PR and between 13242cc and 999231d.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (72)
  • .github/workflows/installer-smoke.yml
  • Cargo.toml
  • app/src-tauri/src/imessage_scanner/mod.rs
  • docs/superpowers/plans/2026-04-21-imessage-live-harness.md
  • docs/superpowers/plans/2026-04-22-life-capture-01-foundation.md
  • docs/superpowers/plans/2026-04-22-track1-ship-pipeline.md
  • docs/superpowers/specs/2026-04-22-life-capture-layer-design.md
  • scripts/fixtures/latest.json
  • scripts/install.sh
  • scripts/test_install.sh
  • src/core/all.rs
  • src/core/jsonrpc.rs
  • src/openhuman/agent/agents/archivist/prompt.rs
  • src/openhuman/agent/agents/code_executor/prompt.rs
  • src/openhuman/agent/agents/critic/prompt.rs
  • src/openhuman/agent/agents/help/prompt.rs
  • src/openhuman/agent/agents/integrations_agent/prompt.rs
  • src/openhuman/agent/agents/loader.rs
  • src/openhuman/agent/agents/morning_briefing/prompt.rs
  • src/openhuman/agent/agents/orchestrator/prompt.rs
  • src/openhuman/agent/agents/planner/prompt.rs
  • src/openhuman/agent/agents/researcher/prompt.rs
  • src/openhuman/agent/agents/summarizer/prompt.rs
  • src/openhuman/agent/agents/tool_maker/prompt.rs
  • src/openhuman/agent/agents/tools_agent/prompt.rs
  • src/openhuman/agent/agents/trigger_reactor/prompt.rs
  • src/openhuman/agent/agents/trigger_triage/prompt.rs
  • src/openhuman/agent/agents/welcome/prompt.rs
  • src/openhuman/agent/debug/mod.rs
  • src/openhuman/agent/harness/fork_context.rs
  • src/openhuman/agent/harness/session/builder.rs
  • src/openhuman/agent/harness/session/runtime.rs
  • src/openhuman/agent/harness/session/turn.rs
  • src/openhuman/agent/harness/session/types.rs
  • src/openhuman/agent/harness/subagent_runner/ops.rs
  • src/openhuman/agent/prompts/mod.rs
  • src/openhuman/agent/prompts/types.rs
  • src/openhuman/agent/triage/escalation.rs
  • src/openhuman/agent/triage/evaluator.rs
  • src/openhuman/curated_memory/mod.rs
  • src/openhuman/curated_memory/rpc.rs
  • src/openhuman/curated_memory/runtime.rs
  • src/openhuman/curated_memory/schemas.rs
  • src/openhuman/curated_memory/store.rs
  • src/openhuman/curated_memory/types.rs
  • src/openhuman/learning/prompt_sections.rs
  • src/openhuman/life_capture/embedder.rs
  • src/openhuman/life_capture/index.rs
  • src/openhuman/life_capture/ingest/composio.rs
  • src/openhuman/life_capture/ingest/mod.rs
  • src/openhuman/life_capture/migrations.rs
  • src/openhuman/life_capture/migrations/0001_init.sql
  • src/openhuman/life_capture/migrations/0002_vec.sql
  • src/openhuman/life_capture/mod.rs
  • src/openhuman/life_capture/quote_strip.rs
  • src/openhuman/life_capture/redact.rs
  • src/openhuman/life_capture/rpc.rs
  • src/openhuman/life_capture/runtime.rs
  • src/openhuman/life_capture/schemas.rs
  • src/openhuman/life_capture/tests/e2e.rs
  • src/openhuman/life_capture/tests/mod.rs
  • src/openhuman/life_capture/types.rs
  • src/openhuman/mod.rs
  • src/openhuman/tools/impl/life_capture/mod.rs
  • src/openhuman/tools/impl/life_capture/search.rs
  • src/openhuman/tools/impl/mod.rs
  • src/openhuman/tools/ops.rs
  • tests/agent_harness_public.rs
  • tests/composio_ingest_e2e.rs
  • tests/fixtures/life_capture/corpus.json
  • tests/imessage_ingest_e2e.rs
  • tests/life_capture_retrieval_eval.rs

Comment thread scripts/install.sh Outdated
Comment on lines 369 to 371
resolve_from_release_api
resolve_rc=$?
if [ "${resolve_rc}" -ne 0 ]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

set -e will short-circuit the new rc-2 fallback — script exits before resolve_rc is captured.

The script is run under set -euo pipefail (Line 6). With set -e active, a bare resolve_from_release_api statement that returns nonzero terminates the script immediately — Bash only suppresses errexit when the command is in an if/while/until condition, in a &&/|| chain, or inverted with !. So when the function hits its return 1 (curl/parse failed) or return 2 (no compatible asset) paths, line 370 (resolve_rc=$?) and the new dry-run branching at 372-388 are unreachable.

This regresses the very behaviour you added — a Linux user without a published asset can't even dry-run because the script dies at line 369 instead of falling into the friendly DRY RUN: no compatible artifact available yet exit.

🐛 Restore the errexit-safe call form
-  resolve_from_release_api
-  resolve_rc=$?
+  resolve_rc=0
+  resolve_from_release_api || resolve_rc=$?
   if [ "${resolve_rc}" -ne 0 ]; then

Or, equivalently, keep the explicit guarded form:

-  resolve_from_release_api
-  resolve_rc=$?
-  if [ "${resolve_rc}" -ne 0 ]; then
+  if ! resolve_from_release_api; then
+    resolve_rc=$?
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
resolve_from_release_api
resolve_rc=$?
if [ "${resolve_rc}" -ne 0 ]; then
resolve_rc=0
resolve_from_release_api || resolve_rc=$?
if [ "${resolve_rc}" -ne 0 ]; then
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/install.sh` around lines 369 - 371, The call to
resolve_from_release_api is currently a bare command under set -e so the script
will exit on nonzero returns before resolve_rc is captured; change the
invocation to an errexit-safe form (e.g., call it inside an if condition like
`if resolve_from_release_api; then ... fi` or append `|| true` so the function
can return a nonzero code without exiting), then capture its exit code into
resolve_rc and let the existing dry-run/DRY RUN messaging branch run when
resolve_rc != 0; update the invocation site around resolve_from_release_api and
the resolve_rc handling so the dry-run branch (the code that prints "DRY RUN: no
compatible artifact available yet") is reachable.

Comment thread src/core/jsonrpc.rs
Comment on lines +949 to +989
let api_key = std::env::var("OPENHUMAN_EMBEDDINGS_KEY")
.or_else(|_| std::env::var("OPENAI_API_KEY"))
.ok();
let Some(api_key) = api_key else {
log::info!(
"[life_capture] embedder not configured (set OPENAI_API_KEY or \
OPENHUMAN_EMBEDDINGS_KEY); life_capture.search will return \
'embedder not configured', life_capture.get_stats remains available"
);
return;
};

let base_url = std::env::var("OPENHUMAN_EMBEDDINGS_URL")
.unwrap_or_else(|_| "https://api.openai.com/v1".into());
let model = std::env::var("OPENHUMAN_EMBEDDINGS_MODEL")
.unwrap_or_else(|_| "text-embedding-3-small".into());
let embedder: Arc<dyn crate::openhuman::life_capture::embedder::Embedder> = Arc::new(
crate::openhuman::life_capture::embedder::HostedEmbedder::new(
base_url,
api_key,
model.clone(),
),
);
if let Err(e) =
crate::openhuman::life_capture::runtime::init_embedder(Arc::clone(&embedder)).await
{
log::debug!("[life_capture] embedder init skipped: {e}");
} else {
log::info!("[life_capture] embedder initialised — model={model}");
}

// Alongside the existing `register_composio_trigger_subscriber()` (triage +
// history), install a second subscriber that routes Gmail + Google Calendar
// Composio triggers into the PersonalIndex. Safe to call here because both
// the index and embedder are now known-good, and the event bus registrar
// is idempotent.
crate::openhuman::life_capture::ingest::register_life_capture_composio_bridge(
Arc::clone(&idx),
embedder,
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Composio Gmail/Calendar ingest is silently disabled when no embeddings key is set.

The early return at Line 958 short-circuits before register_life_capture_composio_bridge (Line 985), so on installs without OPENAI_API_KEY / OPENHUMAN_EMBEDDINGS_KEY the A2 bridge is never wired up — Composio triggers for GMAIL_NEW_GMAIL_MESSAGE etc. flow past life_capture entirely. The info log at Lines 953-957 only mentions search; it doesn't tell the user that ingest is also off, so this looks like the bridge "doesn't work" rather than "needs an embedder."

Suggest tightening the log to call out both failure modes so operators aren't surprised:

📝 Tighter log message
     let Some(api_key) = api_key else {
         log::info!(
             "[life_capture] embedder not configured (set OPENAI_API_KEY or \
-             OPENHUMAN_EMBEDDINGS_KEY); life_capture.search will return \
-             'embedder not configured', life_capture.get_stats remains available"
+             OPENHUMAN_EMBEDDINGS_KEY); life_capture.search will return \
+             'embedder not configured', life_capture.get_stats remains available, \
+             and the Composio Gmail/Calendar bridge will NOT be registered \
+             (events will be dropped at the bus until an embedder is configured)"
         );
         return;
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/core/jsonrpc.rs` around lines 949 - 989, The current early return when
OPENHUMAN_EMBEDDINGS_KEY / OPENAI_API_KEY is missing prevents
register_life_capture_composio_bridge from being called, silently disabling
Composio Gmail/Calendar ingest; instead, keep the bridge wired regardless of an
embedder by moving the call to
crate::openhuman::life_capture::ingest::register_life_capture_composio_bridge(Arc::clone(&idx),
embedder_optional) out of the early-return path (or create a no-op LocalEmbedder
placeholder and still construct/embed it) and change the log message around the
api_key check to clearly state that the embedder is not configured but the
Composio bridge remains registered (or will use a noop embedder) so ingest still
flows to life_capture even when embeddings are not configured.

Comment on lines +21 to +25
let http = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(30))
.connect_timeout(std::time::Duration::from_secs(10))
.build()
.unwrap_or_else(|_| reqwest::Client::new());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent fallback drops the configured timeouts.

If Client::builder().build() ever fails, the fallback reqwest::Client::new() returns a client with no timeouts — meaning a stuck embeddings endpoint can hang request threads indefinitely instead of failing in 30s. Either propagate the error from new (return anyhow::Result<Self>) or expect() here; a default-config fallback for an external-call client is worse than crashing loudly at startup.

🛡️ Proposed fix
-    pub fn new(base_url: String, api_key: String, model: String) -> Self {
-        let http = reqwest::Client::builder()
-            .timeout(std::time::Duration::from_secs(30))
-            .connect_timeout(std::time::Duration::from_secs(10))
-            .build()
-            .unwrap_or_else(|_| reqwest::Client::new());
-        Self {
+    pub fn new(base_url: String, api_key: String, model: String) -> anyhow::Result<Self> {
+        let http = reqwest::Client::builder()
+            .timeout(std::time::Duration::from_secs(30))
+            .connect_timeout(std::time::Duration::from_secs(10))
+            .build()
+            .context("build hosted embedder http client")?;
+        Ok(Self {
             base_url,
             api_key,
             model,
             http,
-        }
+        })
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let http = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(30))
.connect_timeout(std::time::Duration::from_secs(10))
.build()
.unwrap_or_else(|_| reqwest::Client::new());
pub fn new(base_url: String, api_key: String, model: String) -> anyhow::Result<Self> {
let http = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(30))
.connect_timeout(std::time::Duration::from_secs(10))
.build()
.context("build hosted embedder http client")?;
Ok(Self {
base_url,
api_key,
model,
http,
})
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/embedder.rs` around lines 21 - 25, The current
construction of the reqwest client assigns to http using
reqwest::Client::builder().build().unwrap_or_else(|_| reqwest::Client::new()),
which silently drops the configured timeouts if build() fails; change this so
the error is not swallowed — either propagate the build error by returning
anyhow::Result (update the enclosing constructor function to return Result and
bubble up the reqwest::Error) or replace unwrap_or_else with expect() so the
process fails loudly on startup; locate the http assignment
(reqwest::Client::builder(), .timeout(), .connect_timeout(), .build()) in
embedder.rs and update the surrounding function signature/return path
accordingly.

Comment on lines +55 to +75
async fn ingest_gmail(&self, payload: &Value) {
let Some((external_id, ts, subject, text, metadata)) = normalize_gmail(payload) else {
tracing::debug!(
"[life_capture:composio] gmail payload missing required fields — skipping"
);
return;
};
self.do_ingest(Source::Gmail, external_id, ts, subject, text, metadata)
.await;
}

async fn ingest_calendar(&self, payload: &Value) {
let Some((external_id, ts, subject, text, metadata)) = normalize_calendar(payload) else {
tracing::debug!(
"[life_capture:composio] calendar payload missing required fields — skipping"
);
return;
};
self.do_ingest(Source::Calendar, external_id, ts, subject, text, metadata)
.await;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether redaction/quote-strip is applied on the ingest path before
# items hit SQLite. Look for a redaction module and any callers from rpc.rs / index.rs.
fd -t f -e rs 'redact|pii|quote' src/openhuman/life_capture
rg -nP -C3 '(redact|pii|strip_quote|quote_strip)\s*\(' src/openhuman/life_capture
ast-grep --pattern 'pub async fn handle_ingest($$$) { $$$ }'
ast-grep --pattern 'pub async fn upsert($$$) -> $_ { $$$ }'

Repository: tinyhumansai/openhuman

Length of output: 11884


🏁 Script executed:

# Find handle_ingest function definition
ast-grep --pattern 'pub async fn handle_ingest($_$, $_$, $_$, $_$, $_$, $_$) { $$$ }'

# Check normalize_gmail and normalize_calendar implementations
rg -n 'fn normalize_gmail|fn normalize_calendar' src/openhuman/life_capture

# Check do_ingest implementation in composio.rs
rg -n -A15 'async fn do_ingest' src/openhuman/life_capture/ingest/composio.rs

Repository: tinyhumansai/openhuman

Length of output: 774


🏁 Script executed:

# Get full do_ingest implementation
sed -n '77,105p' src/openhuman/life_capture/ingest/composio.rs

# Get handle_ingest function signature and initial implementation
rg -n -A30 'async fn handle_ingest' src/openhuman/life_capture/rpc.rs

# Check if redaction is called anywhere in the do_ingest path
rg -n 'redact\|quote_strip' src/openhuman/life_capture/ingest/composio.rs

Repository: tinyhumansai/openhuman

Length of output: 1734


🏁 Script executed:

# Get the rest of handle_ingest after line 152
sed -n '123,200p' src/openhuman/life_capture/rpc.rs

# Check if redaction happens in life_capture module initialization or somewhere in the pipeline
rg -n 'redact\|quote_strip' src/openhuman/life_capture/rpc.rs

Repository: tinyhumansai/openhuman

Length of output: 1595


Apply redaction and quote-stripping before text is persisted.

normalize_gmail extracts the raw email body and passes it through do_ingest → handle_ingest → IndexWriter::upsert without any PII redaction or quoted-reply stripping. The redact and quote_strip modules exist and are unit-tested, but they are not wired into this ingest path; consequently, raw email bodies (including quoted reply chains and any incidentally-leaked secrets) persist to SQLite unredacted.

Redaction must be applied in the composio ingest flow before IndexWriter::upsert is called, consistent with the coding guideline: "Never log secrets, raw JWTs, API keys, credentials, or full PII."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/ingest/composio.rs` around lines 55 - 75, The
Gmail/Calendar ingest flow currently passes raw text from
normalize_gmail/normalize_calendar through do_ingest → handle_ingest →
IndexWriter::upsert without redaction or quoted-reply stripping; update the
ingest path to run the extracted text (and any text-like metadata) through the
quote_strip and redact utilities before it reaches IndexWriter::upsert.
Concretely, amend ingest_gmail/ingest_calendar or centralize in
do_ingest/handle_ingest to call quote_strip::strip (or equivalent) first, then
redact::redact (or equivalent) on the resulting string(s), and pass the
sanitized subject/text/metadata onwards so IndexWriter::upsert never receives
raw unredacted content. Ensure you reference normalize_gmail/normalize_calendar
inputs and preserve timestamps/external_id while replacing text fields with the
sanitized versions.

Comment on lines +604 to +623
#[test]
fn bridge_has_stable_name_and_domain() {
// Construct with stubs just to exercise the trait methods.
struct Stub;
#[async_trait]
impl Embedder for Stub {
async fn embed_batch(&self, _t: &[&str]) -> anyhow::Result<Vec<Vec<f32>>> {
Ok(vec![])
}
fn dim(&self) -> usize {
1536
}
}
// Lightweight non-init path — we don't actually need a live index
// for the trait introspection bits, but we do need a real
// PersonalIndex handle. Reuse the fresh_bridge pattern via a
// smoke check in the runtime test.
let name = "life_capture::composio_bridge";
assert_eq!(name, name); // shape guard for the name constant
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

This test does not actually verify name() or domains() on the bridge.

The body just constructs an unused local name string and asserts assert_eq!(name, name) — a tautology. The Stub Embedder impl is also dead code. Either drop the test or rewrite it to call the trait methods on a real LifeCaptureComposioBridge instance (e.g. via fresh_bridge().await).

🧪 Proposed rewrite
-    #[test]
-    fn bridge_has_stable_name_and_domain() {
-        // Construct with stubs just to exercise the trait methods.
-        struct Stub;
-        #[async_trait]
-        impl Embedder for Stub {
-            async fn embed_batch(&self, _t: &[&str]) -> anyhow::Result<Vec<Vec<f32>>> {
-                Ok(vec![])
-            }
-            fn dim(&self) -> usize {
-                1536
-            }
-        }
-        // Lightweight non-init path — we don't actually need a live index
-        // for the trait introspection bits, but we do need a real
-        // PersonalIndex handle. Reuse the fresh_bridge pattern via a
-        // smoke check in the runtime test.
-        let name = "life_capture::composio_bridge";
-        assert_eq!(name, name); // shape guard for the name constant
-    }
+    #[tokio::test]
+    async fn bridge_has_stable_name_and_domain() {
+        let bridge = fresh_bridge().await;
+        assert_eq!(bridge.name(), "life_capture::composio_bridge");
+        assert_eq!(bridge.domains(), Some(&["composio"][..]));
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[test]
fn bridge_has_stable_name_and_domain() {
// Construct with stubs just to exercise the trait methods.
struct Stub;
#[async_trait]
impl Embedder for Stub {
async fn embed_batch(&self, _t: &[&str]) -> anyhow::Result<Vec<Vec<f32>>> {
Ok(vec![])
}
fn dim(&self) -> usize {
1536
}
}
// Lightweight non-init path — we don't actually need a live index
// for the trait introspection bits, but we do need a real
// PersonalIndex handle. Reuse the fresh_bridge pattern via a
// smoke check in the runtime test.
let name = "life_capture::composio_bridge";
assert_eq!(name, name); // shape guard for the name constant
}
#[tokio::test]
async fn bridge_has_stable_name_and_domain() {
let bridge = fresh_bridge().await;
assert_eq!(bridge.name(), "life_capture::composio_bridge");
assert_eq!(bridge.domains(), Some(&["composio"][..]));
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/ingest/composio.rs` around lines 604 - 623, The
test currently asserts a tautology and never calls the bridge methods; replace
the dead Stub and the local `name` tautology with an actual bridge instance (use
`fresh_bridge().await` or whichever async helper constructs
`LifeCaptureComposioBridge`) and call `name()` and `domains()` on that instance,
asserting `bridge.name() == "life_capture::composio_bridge"` and that
`bridge.domains()` contains the expected domain(s) (or equals the expected
slice), ensuring the test is async if needed and awaits `fresh_bridge()`; remove
the unused `Stub` impl and any unused imports.

Comment on lines +188 to +193
FieldSchema {
name: "metadata",
ty: TypeSchema::Option(Box::new(TypeSchema::String)),
comment: "Optional JSON object with source-specific metadata.",
required: false,
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

metadata schema type misrepresents the actual contract.

The metadata field is declared as TypeSchema::Option(Box::new(TypeSchema::String)), but the handler at lines 254‑257 reads it as an arbitrary Value (defaulting to an empty Object), and the comment itself says "Optional JSON object". Clients introspecting the schema will be told to send a JSON string, but the handler will silently drop a quoted-string value into the index without parsing it (and conversely accepts objects/arrays the schema disallows). Either change the type to a JSON object/value variant or have the handler parse a JSON string.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/life_capture/schemas.rs` around lines 188 - 193, The schema
declares the FieldSchema for "metadata" as
TypeSchema::Option(Box::new(TypeSchema::String)) but the request handler reads
it as a serde_json::Value (defaulting to an empty Object), so change the schema
to reflect the actual contract: update the FieldSchema for "metadata" to
TypeSchema::Option(Box::new(TypeSchema::Value)) or a concrete JSON object
variant so clients know to send JSON, and ensure the handler still uses the
serde_json::Value path; alternatively, if you prefer to keep TypeSchema::String,
modify the handler code that reads "metadata" to parse the incoming string into
serde_json::Value (and validate errors) instead of accepting arbitrary objects —
update whichever of FieldSchema::metadata or the handler's parsing logic you
choose so schema and handler agree.

Comment on lines +67 to +71
#[allow(clippy::cast_possible_truncation)]
let k = args
.get("k")
.and_then(Value::as_u64)
.map_or(10, |v| v as usize);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

k is not actually capped at 100, contradicting the schema and description.

The parameters_schema declares "maximum": 100 and the description tells the model "capped at 100", but the parsing on Lines 67-71 forwards whatever u64 the model sends straight to handle_search. Schemas are advisory for LLM tool callers — the runtime needs to enforce the cap, otherwise a stray k: 100000 makes the embedder + hybrid_search do unbounded work.

♻️ Proposed change
-        #[allow(clippy::cast_possible_truncation)]
-        let k = args
-            .get("k")
-            .and_then(Value::as_u64)
-            .map_or(10, |v| v as usize);
+        let k = args
+            .get("k")
+            .and_then(Value::as_u64)
+            .map_or(10, |v| v.clamp(1, 100) as usize);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[allow(clippy::cast_possible_truncation)]
let k = args
.get("k")
.and_then(Value::as_u64)
.map_or(10, |v| v as usize);
let k = args
.get("k")
.and_then(Value::as_u64)
.map_or(10, |v| v.clamp(1, 100) as usize);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/openhuman/tools/impl/life_capture/search.rs` around lines 67 - 71, The
code reads k from args into variable k (via
args.get("k").and_then(Value::as_u64).map_or(10, |v| v as usize)) but does not
enforce the documented/schema cap of 100; update the parsing so k is clamped to
the range [1,100] before calling handle_search (e.g., read the u64, convert
safely, then apply min/max/clamp) to ensure the value respect the
parameters_schema and prevents unbounded work by handle_search/hybrid_search.

Comment on lines +320 to +332
{
"id": "q-src-01",
"text": "phoenix",
"kind": "mixed",
"k": 5,
"sources": ["calendar"],
"pending": true,
"pending_reason": "hybrid_search ignores Query.sources/since/until today; runner skips assertions until the implementation branch honors the filter.",
"must_contain_in_top": 5,
"must_contain": ["phoenix-kickoff-cal-01"],
"must_not_contain_in_top": 5,
"must_not_contain": ["phoenix-postmortem-01", "phoenix-retro-slack-01", "phoenix-budget-01", "phoenix-demo-push-imsg-01"]
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

q-src-01 negative set is incomplete.

Once the source filter is implemented and the pending gate is dropped, this query will assert that calendar-only results exclude phoenix-postmortem-01, phoenix-retro-slack-01, phoenix-budget-01, and phoenix-demo-push-imsg-01 — but it omits phoenix-weather-noise-01 (gmail, subject "Weekend forecast: Phoenix, Arizona"), which is also a non-calendar Phoenix-bearing item. The assertion will pass even if the filter erroneously lets through that gmail row.

🛠️ Suggested fix
       "must_not_contain_in_top": 5,
-      "must_not_contain": ["phoenix-postmortem-01", "phoenix-retro-slack-01", "phoenix-budget-01", "phoenix-demo-push-imsg-01"]
+      "must_not_contain": ["phoenix-postmortem-01", "phoenix-retro-slack-01", "phoenix-budget-01", "phoenix-demo-push-imsg-01", "phoenix-weather-noise-01"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/fixtures/life_capture/corpus.json` around lines 320 - 332, The negative
set for the test case with id "q-src-01" is missing an additional non-calendar
Phoenix item; update the object for q-src-01 to add "phoenix-weather-noise-01"
to the must_not_contain array (and keep must_not_contain_in_top as-is) so the
query will correctly assert that calendar-only results exclude that Gmail item
once the pending gate is removed.

Comment on lines +159 to +167
// Stats — should show exactly one item.
let env_stats1 = post_rpc(&base, "openhuman.life_capture_get_stats", json!({})).await;
let stats1 = rpc_result_body(&env_stats1);
assert_eq!(
stats1["total_items"],
json!(1),
"expected total_items=1 after first ingest, got {stats1}"
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

total_items == 1 assertion is brittle when the OnceCell index is shared across tests.

Both #[ignore]d tests in this binary share the same process-global PersonalIndex (per the file header). Cargo runs integration tests in alphabetical order by default — today ingest_then_search... happens to sort before search_response_shape..., so test 1 sees an empty index. But that's an implicit ordering contract: anyone who renames a test, adds a third one, or runs with a different harness can flip ordering and break this assert with no useful diagnostic ("expected 1, got 2"). Test 2's seed (shape-guard:2026-04-22) is exactly the kind of sibling state that makes this hazard real.

Recommend taking a baseline before the first ingest and asserting the delta, e.g.:

♻️ Proposed change
+    // Baseline — other tests in this binary may have seeded the shared index.
+    let env_stats0 = post_rpc(&base, "openhuman.life_capture_get_stats", json!({})).await;
+    let baseline = rpc_result_body(&env_stats0)["total_items"].as_i64().unwrap_or(0);
+
     // First ingest — should INSERT.
     let env1 = post_rpc(
         /* … */
     ).await;
     /* … */

-    // Stats — should show exactly one item.
+    // Stats — should show exactly one new item over baseline.
     let env_stats1 = post_rpc(&base, "openhuman.life_capture_get_stats", json!({})).await;
     let stats1 = rpc_result_body(&env_stats1);
-    assert_eq!(
-        stats1["total_items"],
-        json!(1),
-        "expected total_items=1 after first ingest, got {stats1}"
-    );
+    assert_eq!(
+        stats1["total_items"].as_i64().unwrap(),
+        baseline + 1,
+        "expected total_items=baseline+1 after first ingest, got {stats1}"
+    );

The later "stats unchanged on re-ingest" check (Lines 217-220) is already delta-based and robust — this just brings the first assertion in line.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/imessage_ingest_e2e.rs` around lines 159 - 167, The assertion that
stats1["total_items"] == 1 is brittle due to shared process-global
PersonalIndex; before performing the first ingest, fetch a baseline with
post_rpc(&base, "openhuman.life_capture_get_stats", json!({})) (reuse
rpc_result_body to parse) and record baseline_total =
stats_baseline["total_items"], then after ingest compute delta =
stats1["total_items"] - baseline_total and assert delta == 1; update the test to
use these symbols (env_stats_baseline, stats_baseline, baseline_total,
env_stats1, stats1) so the test asserts a delta rather than an absolute total.

@jwalin-shah
jwalin-shah force-pushed the feat/a2-composio-bridge branch from 999231d to e4c6a87 Compare May 4, 2026 06:50
jwalinsshah and others added 6 commits May 4, 2026 12:04
…xtures

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…uplicates

- Introduce CuratedMemoryPromptSnapshot in prompt types so CI builds without
  the curated_memory domain still resolve PromptContext.
- Use that type for ParentExecutionContext, Agent session storage, and
  PromptContext; map from curated_memory::MemorySnapshot when taking snapshots.
- Remove duplicate curated_snapshot fields from bad merges (struct literals
  and PromptContext construction).
- Fix learning tests PromptContext initializer.

Co-authored-by: Cursor <cursoragent@cursor.com>
CI failed with E0063 after ParentExecutionContext gained curated_snapshot;
triage escalation, session turn builder, subagent tests, and integration
stubs were still missing the field.

Co-authored-by: Cursor <cursoragent@cursor.com>
@senamakel senamakel closed this May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants